Document structured concurrency #4433

dkhalanskyjb · 2025-05-09T12:08:52Z

Draft for now, because the documentation of some other functions should be updated accordingly:

The remaining entries in CoroutineScope.kt.
supervisorScope
Builders.common.kt: launch, async, and the other coroutine builders should document their structured concurrency behaviors.
The withTimeout family of functions.
produce.
Coroutine builders from the reactive integrations.
JS+Wasm promise.
The future function.

but reviewing the changes should already be okay (I'll introduce further changes in separate commits).

cc @LouisCAD, given your real-world experience and interest in the documentation, your comments on this would be greatly appreciated!

murfel

Let me know if it's too nitpicky. Or if you'd prefer the first round to be more high-level, and then you'd be interested in nitpicky to polish it up. Your feedback to my feedback is appreciated 🙂

murfel · 2025-07-14T09:25:14Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

- * thus enforcing the structured concurrency. See [Job] documentation for more details.
+ * The methods of this interface are not intended to be called directly.
+ * Instead, a [CoroutineScope] is passed as a receiver to the coroutine builders such as [launch] and [async],
+ * controlling the execution properties and lifetimes of the created coroutines.


lifetimes -> lifecycles

Could you elaborate?

murfel · 2025-07-14T09:26:36Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

- * The [CoroutineScope.cancel] extension function shall be used when the entity that was launching coroutines
- * is no longer needed. It cancels all the coroutines that might still be running on behalf of it.
+ * Manual implementations of this interface are not recommended.
+ * Instead, there are several structured ways to obtain a [CoroutineScope].


~~structured~~

murfel · 2025-07-14T09:30:51Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ *   Some commonly used dispatchers are provided in the [Dispatchers] object.
+ * - [CoroutineExceptionHandler] that defines how failures of child coroutines should be reported whenever
+ *   structured concurrency does not provide a way to propagate the failure to the parent
+ *   (typically, because the root scope of the ancestry tree is not lexically scoped).


fyi: this is where I had to lookup what "lexically scoped" means, my understanding from the google summary is "where / in which context it is defined / written in the source code"

but this still leaves me wondering what are the specific cases this "not lexically scoped" refers to

I managed to get the gist of "lexically" after reading this section, but I stumbled every time I saw this word.

It only gets clear after the Lexical scopes and CoroutineScope constructor sections.

Possibly define it, avoid using before these sections, or refer to those sections?

murfel · 2025-07-14T10:34:54Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ * [coroutineScope] and [supervisorScope] functions can be used in any `suspend` function to define a scope
+ * lexically, ensuring that all coroutines launched in this scope have completed by the time scope-limiting
+ * function exits.


Split the sentence in two to simplify reading. Less ideas per sentence.

Suggested change

* [coroutineScope] and [supervisorScope] functions can be used in any `suspend` function to define a scope

* lexically, ensuring that all coroutines launched in this scope have completed by the time scope-limiting

* function exits.

* [coroutineScope] and [supervisorScope] functions can be used in any `suspend` function to define a scope

* lexically. All coroutines launched in the new scope will have completed by the time the scope-limiting

* function exits.

Maybe it's worth avoiding the new term "scope-limiting function". Maybe "by the time the lexical scope function exits"?

Less ideas per sentence is good, but is it really okay to have less than one idea per sentence? In my view, it's better to have a longer sentence that describes a single idea instead of having two sentences with 0.5 ideas each.

the new term "scope-limiting function". Maybe "by the time the lexical scope function exits"?

"The lexical scope function" is also a new term, though. What makes a function lexical-scope? Fully stated, coroutineScope is a function that limits the coroutine scope to the lexical scope of its block, yet this is a mouthful and distracts from the idea that's being conveyed. Do you have an idea of how this could be expressed clearly but also succinctly?

murfel · 2025-07-14T11:44:57Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ * } // will only exit once all `Coroutine X finished` messages are printed
+ * ```
+ *
+ * This should be the preferred way to create a scope for coroutines.


Move it before the example? So that there's no confusion if it refers to this section, or if it's a setup for the next section.

Is it confusing in the rendered form as well? It's expected that this text will be read from kotlinlang.org or from the IDE in the popup window that appears on a mouse-over.

murfel · 2025-07-14T12:06:10Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ *
+ * This should be the preferred way to create a scope for coroutines.
+ *
+ * ### `CoroutineScope` constructor function


Shouldn't it just be called constructor, instead of constructor function?

CoroutineScope is not a class and does not have a constructor, we only provide a constructor function.

murfel · 2025-07-14T12:27:09Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ * It is convenient for launching top-level coroutines that are not tied to the lifecycle of any entity,
+ * but it is easy to misuse it and create memory leaks or resource leaks when a coroutine actually should be tied
+ * to the lifecycle of some entity.


Simplify by splitting into two, less ideas per sentence.

However, it is easy to misuse...

(Let me know if you'd like me to point out other occurrences of this in my future reading bouts of this PR.)

(By the way, prompting an LLM with "rewrite this sentence" gives out the same sentence split that what I would suggest, and some stylistically better words that I wouldn't come up with myself. Although it needs some editing to put all the erased terms back in.)

murfel · 2025-07-14T12:40:32Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ * ```
+ * GlobalScope.launch(CoroutineExceptionHandler { _, e ->
+ *     println("Error in coroutine: $e")
+ * }) {
+ *    while (true) {
+ *        println("I will be running forever, you cannot stop me!")
+ *        delay(1.seconds)
+ *    }
+ * }
+ * ```


I feel like it's more of an example, unrelated to GlobalScope, for "if you define your custom exception handler which ignores exceptions, no one will be able to cancel you", rather than "if you misuse GlobalScope, you're setting yourself up for failure".

Also it relies on the hidden curriculum of "cancelation is implemented as an exception" which I believe hasn't been covered up to here yet.

if you define your custom exception handler which ignores exceptions, no one will be able to cancel you

That's not true, though. The CoroutineExceptionHandler is here just to solidify the good practice of supplying them. Removing the CoroutineExceptionHandler does not change the point.

murfel · 2025-07-14T12:46:05Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ * When all else fails and a custom [CoroutineScope] implementation is needed, it is recommended to use
+ * `by`-delegation to implement the interface:


Fails is a very specific word and brings unneeded connotations.

When no other approaches are applicable

murfel · 2025-07-14T12:49:53Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ * }
+ * ```
+ *
+ * ### `by`-delegation


I'm a bit surprised that I only learn that I should use by-delegration instead of the constructor [function] at the very end of the section. Possibly mention in the beginning that the preferred approach is the by-delegation, and the constructor is only spelled out there for clarity/demonstration?

The example is using the constructor function here to obtain a non-custom implementation. The constructor function is better than by-delegation, which is in turn better than override val coroutineContext: CoroutineContext.

dkhalanskyjb · 2025-07-14T12:57:49Z

Higher-level overview first is nice, since there's no point in debating the grammar of a section that gets removed as a whole. So, the nitpicking should (initially) be limited to exactly the level above which everything's already well done.

dkhalanskyjb · 2025-07-14T13:00:21Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ *   Some commonly used dispatchers are provided in the [Dispatchers] object.
+ * - [CoroutineExceptionHandler] that defines how failures of child coroutines should be reported whenever
+ *   structured concurrency does not provide a way to propagate the failure to the parent
+ *   (typically, because the root scope of the ancestry tree is not lexically scoped).


Would a specific example make it clear?

Suggested change

* (typically, because the root scope of the ancestry tree is not lexically scoped).

* (typically, because the root scope of the ancestry tree is not lexically scoped,

that is, not created using builders like [coroutineScope] or [withContext]

that return the result directly to the caller).

dkhalanskyjb · 2025-07-14T13:02:36Z

kotlinx-coroutines-core/common/src/CoroutineScope.kt

+ * It is convenient for launching top-level coroutines that are not tied to the lifecycle of any entity,
+ * but it is easy to misuse it and create memory leaks or resource leaks when a coroutine actually should be tied
+ * to the lifecycle of some entity.


Splitting removes the focus on the contradiction, which is the whole point. Would this work instead?

Suggested change

* It is convenient for launching top-level coroutines that are not tied to the lifecycle of any entity,

* but it is easy to misuse it and create memory leaks or resource leaks when a coroutine actually should be tied

* to the lifecycle of some entity.

* Although is convenient for launching top-level coroutines that are not tied to the lifecycle of any entity,

* it is easy to misuse it and create memory leaks or resource leaks when a coroutine actually should be tied

* to the lifecycle of some entity.

dkhalanskyjb requested a review from qwwdfsad May 9, 2025 12:08

dkhalanskyjb added 5 commits May 21, 2025 11:10

Document structured concurrency

8b321e4

Document the remainder of CoroutineScope.kt

d00844d

Document supervisorScope

6515270

Document CoroutineScope.launch

fc6e842

Document runBlocking and newCoroutineContext

4ad4272

dkhalanskyjb force-pushed the dkhalanskyjb/fix-structured-concurrency-docs branch from ea4a81c to 4ad4272 Compare May 21, 2025 09:13

dkhalanskyjb added 3 commits June 11, 2025 13:47

Document the async function

b705e1c

Document withContext and some more pitfalls

33e4c75

Document the withTimeout function

142162c

dkhalanskyjb requested a review from murfel July 9, 2025 11:11

murfel reviewed Jul 14, 2025

View reviewed changes

dkhalanskyjb commented Jul 14, 2025

View reviewed changes

		* When all else fails and a custom [CoroutineScope] implementation is needed, it is recommended to use
		* `by`-delegation to implement the interface:

- *   (typically, because the root scope of the ancestry tree is not lexically scoped).
+ *   (typically, because the root scope of the ancestry tree is not lexically scoped,
+      that is, not created using builders like [coroutineScope] or [withContext]
+      that return the result directly to the caller).

Document structured concurrency #4433

Are you sure you want to change the base?

Document structured concurrency #4433

Uh oh!

Conversation

dkhalanskyjb commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

murfel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkhalanskyjb commented Jul 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dkhalanskyjb commented May 9, 2025 •

edited

Loading